Skip to content

test(runtime): pin array expandos across growth - #9579

Closed
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/9201-array-named-properties
Closed

test(runtime): pin array expandos across growth#9579
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/9201-array-named-properties

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 3, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

  • add a focused runtime regression proving address-keyed named properties are rekeyed when array growth replaces the allocation
  • add Node differential coverage for statically typed and dynamic arrays, including enumeration and JSON behavior
  • associate the existing growth transfer with A named property assigned to an array is silently dropped #9201

Context

The named assignment itself worked; the following indexed write grew the array and historically left its expando table under the old allocation address. The owner-transfer implementation landed on main in #9434 after this issue was opened, so this PR pins that behavior directly and closes the previously unlinked report.

Testing

  • cargo test -p perry-runtime growth_rekeys_named_property_owner -- --nocapture
  • cargo test -p perry-runtime --quiet -- --test-threads=1 (3,011 passed; 4 ignored)
  • PERRY_SKIP_BUILD=1 ./run_parity_tests.sh --filter 9201 (1/1 passed against Node 26.5.1)
  • ./scripts/pre-tag-check.sh --quick
  • python3 scripts/check_test_registration.py
  • cargo fmt --all -- --check
  • git diff --check

No version bump is included.

Closes #9201

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where named properties on arrays could become inaccessible after writing to an index beyond the current capacity.
    • Named properties now remain available after array growth, including string, object, and boolean values.
    • Property enumeration and JSON serialization now continue to reflect these properties correctly after sparse array writes.

  • Tests

    • Added regression coverage for dynamic and typed array growth scenarios.

@coderabbitai

coderabbitai Bot commented Sep 3, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 463de918-3bfd-4949-a671-85a54124ca04

📥 Commits

Reviewing files that changed from the base of the PR and between 8d27f7b and ba6810b.

📒 Files selected for processing (4)
  • changelog.d/9579-array-expando-growth.md
  • crates/perry-runtime/src/array/large_presized_tests.rs
  • crates/perry-runtime/src/array/push_pop.rs
  • test-files/test_gap_9201_array_named_properties.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.

📝 Walkthrough

Walkthrough

The change adds runtime and TypeScript regression coverage for named properties on arrays after indexed writes grow their backing allocation. It also updates the related ownership comment and adds a changelog entry.

Changes

Array named-property preservation

Layer / File(s) Summary
Array growth regression coverage
crates/perry-runtime/src/array/large_presized_tests.rs, test-files/test_gap_9201_array_named_properties.ts, crates/perry-runtime/src/array/push_pop.rs, changelog.d/9579-array-expando-growth.md		 The tests verify property preservation, side-table ownership transfer, property enumeration, and JSON serialization after sparse writes. The comment and changelog describe the related ownership behavior and issue.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to ba681

This change adds coverage and documentation for preserving named array properties through array growth without changing runtime behavior. The regression coverage protects property access, enumeration, and JSON behavior, and no merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the runtime test and the array-expando growth behavior covered by the changes.
Description check ✅ Passed The description provides a clear summary, implementation context, related issue, detailed test commands and results, and confirms that no version bump is included. It omits some template headings and …
Linked Issues check ✅ Passed The changes address issue #9201 by adding runtime regression coverage and Node differential coverage for named-property persistence, enumeration, sparse writes, and JSON behavior after array growth. T…
Out of Scope Changes check ✅ Passed The changelog entry, runtime regression test, differential test, and issue-reference comment all support the array named-property growth fix. No unrelated code or metadata changes are present.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (1 skipped: 1 …
Full details: Description check

Explanation

The description provides a clear summary, implementation context, related issue, detailed test commands and results, and confirms that no version bump is included. It omits some template headings and checklist items, but it remains mostly complete.

Full details: Linked Issues check

Explanation

The changes address issue #9201 by adding runtime regression coverage and Node differential coverage for named-property persistence, enumeration, sparse writes, and JSON behavior after array growth. The description also confirms that the owner-transfer implementation already exists on main and that this PR pins it to the issue.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug proggeramlug mentioned this pull request Sep 3, 2026
Merged
proggeramlug pushed a commit that referenced this pull request Sep 3, 2026
Ralph Küpper @proggeramlug
docs(changelog): record array expando growth coverage (#9579)
0a02fbd
@proggeramlug

proggeramlug commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Landed via merge train #9581 (rebase-merge, authorship preserved).

@proggeramlug proggeramlug mentioned this pull request Sep 3, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

A named property assigned to an array is silently dropped

1 participant

@proggeramlug